Clipper Manager Python Tests#187
Conversation
|
Test FAILed. |
|
Test FAILed. |
dcrankshaw
left a comment
There was a problem hiding this comment.
This is great! It will be really good to have test coverage of clipper_manager as we start refactoring it.
bin/run_unittests.sh
Outdated
| echo -e "\nRunning management tests\n\n" | ||
| ./src/management/managementtests --redis_port $REDIS_PORT | ||
| cd $DIR | ||
| python ../management/test/clipper_manager_test.py |
There was a problem hiding this comment.
clipper_manager.py got moved from the management/ to the clipper_admin/ directory as part of making the pip package. Move the tests to the same place (clipper_admin/test).
|
|
||
| if __name__ == '__main__': | ||
| run_short = True | ||
| run_long = True |
There was a problem hiding this comment.
There should be a run_all option as well
| self.assertGreaterEqual(len(running_containers_output), 1) | ||
|
|
||
|
|
||
| class ClipperManagerTestCaseLong(unittest.TestCase): |
There was a problem hiding this comment.
Can you import sklearn here so that the short test case can still be run without sklearn installed?
There was a problem hiding this comment.
Both cases requiresklearn. It is required in test_model_deploys_successfully (short case) and test_deployed_model_queried_successfully (long case).
| cur_dir = os.path.dirname(os.path.abspath(__file__)) | ||
| sys.path.insert(0, os.path.abspath('%s/../' % cur_dir)) | ||
| import clipper_manager | ||
|
|
There was a problem hiding this comment.
Can you add a comment at the top of the file briefly describing how these tests are run? Basically just say that at the beginning of each test suite a local Clipper instance is created and the tests issue various commands against the running Clipper instance and verify their results.
This is just so that it's documented that these tests start Docker containers and interact with them, as opposed to being pure Python tests.
|
@dcrankshaw Addressed your comments. |
|
Test FAILed. |
|
Test FAILed. |
dcrankshaw
left a comment
There was a problem hiding this comment.
I pushed a couple really minor cleanup changes.
I commented out the deploy_predict_function tests until #194 gets merged because dealing with conda environments in jenkins is a little messy and I don't want to be blocked on that.
|
Test FAILed. |
|
Test FAILed. |
|
Test FAILed. |
|
Test FAILed. |
|
Test PASSed. |
Test via: